Skip to content

Conversation

@tannewt
Copy link
Member

@tannewt tannewt commented Nov 17, 2025

Shouldn't change functionality at all.

Smoke tested on the nRF52840 DK.

@tannewt
Copy link
Member Author

tannewt commented Nov 18, 2025

FYI I'm working through the CI build issues and will rebase once I figure it out and then request a review.

@tannewt tannewt marked this pull request as ready for review November 21, 2025 22:17
@tannewt
Copy link
Member Author

tannewt commented Nov 21, 2025

Finally got this all sorted. It is ready for review.

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your persistence!

circuitpython_flags.append(f"-DCIRCUITPY_FULL_BUILD={1 if full_build else 0}")
circuitpython_flags.append(f"-DCIRCUITPY_USB_HOST={1 if usb_host else 0}")
circuitpython_flags.append(f'-DCIRCUITPY_BOARD_ID=\\"{board}\\"')
circuitpython_flags.append(f"-DCIRCUITPY_BOARD_ID='\"{board}\"'")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I am just curious. Why doesn't

    circuitpython_flags.append(f"-DCIRCUITPY_BOARD_ID={board}")

work here, since we know the board has no spaces? Does the context in which this is used require a double-quoted argument?

Similarly elsewhere below, where there are filenames in quotes, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filenames are used in #include lines and need the quotes. This needs them to be a c-string:

static const MP_DEFINE_STR_OBJ(board_module_id_obj, CIRCUITPY_BOARD_ID);

I changed it to wrapped in single quotes because the response file will escape the backspace formerly used to escape the double quote.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants